Fpscounter #28
Fpscounter #28
Conversation
Summary of ChangesHello @Mosch0512, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a robust set of tools for in-game performance monitoring, moving beyond simple average FPS to provide deeper insights into frame time consistency. By tracking and visualizing percentile low FPS and individual frame times, developers can more effectively diagnose and address performance bottlenecks, leading to a smoother and more consistent user experience. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive FPS counter with detailed statistics, including average FPS, percentile lows (1% and 0.1%), and a real-time frame time graph. This is a great addition for performance monitoring and debugging. The new functionality is controlled via a console command $fpscounter on/off.
My review focuses on improving code clarity, consistency, and adherence to C++ best practices. I've suggested using standard library algorithms to simplify code, replacing magic numbers with named constants, and improving the consistency of displayed statistics. I've also pointed out a high-severity issue regarding the use of extern declarations within functions, which should be replaced with proper header includes for better modularity.
RenderDebugInfo (FPS, mouse position, camera angles) is now: - Always on in Debug builds - Off by default in Release builds, toggle with $fpscounter on/off The $fpscounter command is checked before the $fps prefix to avoid a crash from trying to parse "ounter on" as a float.
Show avg FPS, max FPS, 1% low, 0.1% low, frame time in ms. Render a color-coded frame time bar graph (green/yellow/red) with a 60fps reference line.
… overlay Address PR review feedback: replace extern declarations with header include, extract magic numbers to named constants, use std::accumulate, fix highest FPS to track instantaneous values, rename misleading 0.1% low to slowest frame, fix frame time consistency.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a simple FPS counter and a detailed performance overlay, complete with a frame time graph. The new features are controllable via chat commands. The implementation is solid, adding valuable debugging and performance monitoring tools. I've included a couple of suggestions to optimize the rendering of the new frame graph and to improve the command parsing logic for better efficiency and maintainability.
Move glBegin/glEnd outside the frame bar loop to reduce draw calls. Use else-if chain in CheckCommand to skip unnecessary comparisons.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a useful FPS counter and a detailed performance overlay, which are great additions for debugging and performance tuning. The implementation is straightforward and the new chat commands are well-integrated. The review includes suggestions to improve maintainability by refactoring the command parsing logic, using named constants instead of magic numbers for UI layout, and considering modern OpenGL practices for rendering the new overlay to align with the project's optimization goals.
| static void RenderFrameGraph(float graphX, float graphY, float graphW, float graphH) | ||
| { | ||
| if (s_frameCount < 2) | ||
| return; | ||
|
|
||
| // Convert virtual 640x480 coords to actual window pixels | ||
| float gx = graphX * (float)WindowWidth / 640.f; | ||
| float gy = graphY * (float)WindowHeight / 480.f; | ||
| float gw = graphW * (float)WindowWidth / 640.f; | ||
| float gh = graphH * (float)WindowHeight / 480.f; | ||
|
|
||
| // Flip Y for OpenGL (origin bottom-left) | ||
| float glBottom = (float)WindowHeight - gy - gh; | ||
| float glTop = (float)WindowHeight - gy; | ||
|
|
||
| // Background | ||
| glDisable(GL_TEXTURE_2D); | ||
| glEnable(GL_BLEND); | ||
| glBlendFunc(GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA); | ||
|
|
||
| glColor4f(0.0f, 0.0f, 0.0f, 0.5f); | ||
| glBegin(GL_QUADS); | ||
| glVertex2f(gx, glBottom); | ||
| glVertex2f(gx + gw, glBottom); | ||
| glVertex2f(gx + gw, glTop); | ||
| glVertex2f(gx, glTop); | ||
| glEnd(); | ||
|
|
||
| // Target line at 16.67ms (60fps) | ||
| float target60 = THRESHOLD_60FPS_MS / GRAPH_MAX_MS; | ||
| float lineY = glBottom + target60 * gh; | ||
| glColor4f(0.3f, 0.8f, 0.3f, 0.5f); | ||
| glBegin(GL_LINES); | ||
| glVertex2f(gx, lineY); | ||
| glVertex2f(gx + gw, lineY); | ||
| glEnd(); | ||
|
|
||
| // Frame bars | ||
| float barW = gw / FRAME_HISTORY_SIZE; | ||
| int oldest = (s_frameCount < FRAME_HISTORY_SIZE) ? 0 : s_frameIndex; | ||
|
|
||
| glBegin(GL_QUADS); | ||
| for (int i = 0; i < s_frameCount; i++) | ||
| { | ||
| int idx = (oldest + i) % FRAME_HISTORY_SIZE; | ||
| float ms = s_frameTimesMs[idx]; | ||
| float norm = std::min(ms / GRAPH_MAX_MS, 1.0f); | ||
| float barH = norm * gh; | ||
|
|
||
| // Color: green < 16.67ms, yellow < 25ms, red >= 25ms | ||
| if (ms < THRESHOLD_60FPS_MS) | ||
| glColor4f(0.2f, 0.9f, 0.2f, 0.8f); | ||
| else if (ms < THRESHOLD_40FPS_MS) | ||
| glColor4f(0.9f, 0.9f, 0.2f, 0.8f); | ||
| else | ||
| glColor4f(0.9f, 0.2f, 0.2f, 0.8f); | ||
|
|
||
| float bx = gx + i * barW; | ||
| glVertex2f(bx, glBottom); | ||
| glVertex2f(bx + barW, glBottom); | ||
| glVertex2f(bx + barW, glBottom + barH); | ||
| glVertex2f(bx, glBottom + barH); | ||
| } | ||
| glEnd(); | ||
|
|
||
| glEnable(GL_TEXTURE_2D); | ||
| } |
There was a problem hiding this comment.
The use of immediate mode OpenGL (glBegin, glEnd) is deprecated and can be inefficient. The pull request description mentions optimizing OpenGL calls with vertex arrays, so it's surprising to see new code using this legacy approach. For better performance and modern practice, this should be refactored to use Vertex Buffer Objects (VBOs) and shaders, if the rendering engine supports it. Even for a simple overlay, using modern OpenGL is preferable for consistency and performance.
src/source/Scenes/SceneManager.cpp
Outdated
| g_pRenderText->RenderText(10, y, szLine); y += 10; | ||
|
|
||
| // Frame time graph below text | ||
| RenderFrameGraph(10.0f, (float)(y + 2), 200.0f, 40.0f); |
There was a problem hiding this comment.
The position and dimensions for the frame graph are hardcoded. Using named constants for these values would improve readability and make it easier to adjust the UI layout in the future.
// At the top of the file or in a relevant header
// static constexpr float DEBUG_GRAPH_X = 10.0f;
// static constexpr float DEBUG_GRAPH_Y_OFFSET = 2.0f;
// static constexpr float DEBUG_GRAPH_WIDTH = 200.0f;
// static constexpr float DEBUG_GRAPH_HEIGHT = 40.0f;
// In RenderDebugInfo()
RenderFrameGraph(10.0f, (float)(y + 2), 200.0f, 40.0f); // Replace with constants| if (strCommand.compare(L"$fpscounter on") == 0) | ||
| { | ||
| SetShowFpsCounter(true); | ||
| return true; | ||
| } | ||
| else if (strCommand.compare(L"$fpscounter off") == 0) | ||
| { | ||
| SetShowFpsCounter(false); | ||
| return true; | ||
| } | ||
| else if (strCommand.compare(L"$details on") == 0) | ||
| { | ||
| SetShowDebugInfo(true); | ||
| return true; | ||
| } | ||
| else if (strCommand.compare(L"$details off") == 0) | ||
| { | ||
| SetShowDebugInfo(false); | ||
| return true; | ||
| } | ||
| else if (strCommand.compare(0, 4, L"$fps") == 0) | ||
| { | ||
| auto fps_str = strCommand.substr(5); | ||
| auto target_fps = std::stof(fps_str); | ||
| SetTargetFps(target_fps); | ||
| return true; | ||
| } | ||
|
|
||
| if (strCommand.compare(L"$vsync on") == 0) | ||
| else if (strCommand.compare(L"$vsync on") == 0) | ||
| { | ||
| EnableVSync(); | ||
| SetTargetFps(-1); // unlimited | ||
| ResetFrameStats(); | ||
| return true; | ||
| } | ||
|
|
||
| if (strCommand.compare(L"$vsync off") == 0) | ||
| else if (strCommand.compare(L"$vsync off") == 0) | ||
| { | ||
| DisableVSync(); | ||
| ResetFrameStats(); | ||
| return true; | ||
| } |
There was a problem hiding this comment.
The if-else if chain for command parsing is becoming long and can be difficult to maintain. A more scalable approach would be to use a std::map to associate command strings with their handler functions. This would make the code cleaner and more extensible.
|
/gemini summary |
Summary of ChangesThis pull request significantly enhances the in-game performance monitoring capabilities by introducing both a basic FPS counter and a comprehensive debug overlay. The detailed overlay provides crucial metrics like 1% low FPS and a visual frame time graph, offering a much deeper understanding of performance consistency beyond just average frame rates. These new tools are easily accessible and controllable via chat commands, enabling developers and users to quickly diagnose and analyze performance characteristics. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
|
1 participant
No description provided.